-
Notifications
You must be signed in to change notification settings - Fork 7.3k
net: add pauseOnConnect option to createServer() #8576
Conversation
Currently, when a server receives a new connection, the underlying socket handle begins reading data immediately. This causes problems when sockets are passed between processes, as data can be read by the first process, and thus never read by the second process. This commit allows sockets that are constructed with a handle to be paused initially.
For completeness, could you paste the IRC chat here maybe? I'm wondering what was wrong with with @trevnorris original suggestion here. I think the idea to hold off starting the underlying handle for a newly connected socket until next-tick after With the approach in this PR, you're forced to understand all the semantics behind this whole mess, in order to know that you need to pass |
I don't have the logs, and it looks like the IRC channel's logs stop at the beginning of the month. My problem with waiting until I don't think it's asking too much for a developer to understand the semantics of this case if they intend to develop around it. I think it's simpler to understand "the socket won't start reading until you tell it to" than it is to understand "the socket will start reading immediately, but not until |
I'm okay with this. It's more explicit than simply delaying on /cc @tjfontaine @indutny Want your feedback as well. @cjihrig If no one has responded by next week ping me. |
@trevnorris here is your next week ping |
this._handle.readStop(); | ||
this._readableState.flowing = false; | ||
} else | ||
this.read(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style nit: if one part of an if
/else
uses brackets all of them should. don't worry about fixing it though. i'll take care of it before merging.
Currently when a server receives a new connection the underlying socket handle begins reading data immediately. This causes problems when sockets are passed between processes, as data can be read by the first process and thus never read by the second process. This commit allows sockets that are constructed with a handle to be paused initially. PR-URL: #8576 Fixes: #7905 Fixes: #7784 Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Thanks much. Merged in c2b4f48. |
Currently, when a server receives a new connection, the underlying socket handle begins reading data immediately. This causes problems when sockets are passed between processes, as data can be read by the first process, and thus never read by the second process. This commit allows sockets that are constructed with a handle to be paused initially.
@trevnorris this is the behavior I referenced on #7905 (comment), which we subsequently discussed on IRC. I plan on looking into UDP sockets next.
Closes #7905. Closes #7784.
UPDATE: After looking into UDP sockets, I don't feel that they suffer from this same problem, as new sockets are not created for each connection.